Mixed deltas plot (rewritten) by fweber144 · Pull Request #511 · stan-dev/projpred

fweber144 · 2025-04-23T10:51:43Z

This is a rewritten version of the mixed deltas plot (#508, originally proposed in #496) that comes with less changes compared to master and should be less error-prone.

@avehtari, could you check if the plots created by this version are ok for you? Especially the axis labels and the plot subtitle would need to be checked (there are a few changes in the labels/subtitle compared to #508).

Here, the new column behavior of the performances() table (which is essentially the summary.vsel() table) implemented in #508 is not included. I would suggest to create a separate PR for that new column behavior if it is still desired (personally, I would prefer not to change it because it has been this way for a long time, already before I started working on projpred).

This PR is not completely ready to be merged. There are a few things I still need to do:

Search for deltas in the whole codebase, especially to ensure that the docs and the vignettes are updated appropriately.
Add new unit tests.
Add a NEWS.md entry.

I would tackle these if @avehtari confirms that the general implementation is ok. If we go with this version, #508 would have to be closed (not merged).

@avehtari

this was proposed by @avehtari in stan-dev#496, but has been rewritten by @fweber144

…_rewritten

avehtari · 2025-05-18T16:24:27Z

With this PR branch, I get an error when using deltas="mixed" with more than stats

+ Error in plot.vsel(cvvs_fast, stats = c("elpd", "R2"), deltas = "mixed",  (from methods.R#771) : 
  Unexpected length of `stats_bs_init$value`. Please report this.

(e.g. using the example in the vignette)

avehtari · 2025-05-18T16:47:42Z

The github PR review system is broken and if I try to make comments specific to certain lines my browser freezes (tested both Chrome and Firefox):

methods.R lines 1110-1125:
This produces a text that this too long if deltas="mixed". If the additional text is shown, there should be newline after the comma, to make the text about the same length as texts below the plot

avehtari · 2025-05-18T16:48:49Z

Here, the new column behavior of the performances() table (which is essentially the summary.vsel() table) implemented in #508 is not included. I would suggest to create a separate PR for that new column behavior if it is still desired (personally, I would prefer not to change it because it has been this way for a long time, already before I started working on projpred).

Without that change to the table, how do you get the values for the intervals corresponding to the plot?

…_rewritten

<stan-dev#511 (comment)>

<stan-dev#511 (comment)> (the check is not necessary in the first place because `stats_bs_init` is a `data.frame`)

fweber144 · 2025-05-20T08:44:57Z

methods.R lines 1110-1125: This produces a text that this too long if deltas="mixed". If the additional text is shown, there should be newline after the comma, to make the text about the same length as texts below the plot

Thanks; fixed in 5638c09.

fweber144 · 2025-05-20T08:45:42Z

With this PR branch, I get an error when using deltas="mixed" with more than stats
+ Error in plot.vsel(cvvs_fast, stats = c("elpd", "R2"), deltas = "mixed",  (from methods.R#771) : 
  Unexpected length of `stats_bs_init$value`. Please report this.
(e.g. using the example in the vignette)

Thanks; fixed in ee57c83.

fweber144 · 2025-05-20T09:04:04Z

Here, the new column behavior of the performances() table (which is essentially the summary.vsel() table) implemented in #508 is not included. I would suggest to create a separate PR for that new column behavior if it is still desired (personally, I would prefer not to change it because it has been this way for a long time, already before I started working on projpred).

Without that change to the table, how do you get the values for the intervals corresponding to the plot?

From the diff columns (as in #508, I have added diff columns diff.lq and diff.uq to .tabulate_stats() output; I have also ensured that argument baseline is respected in that case).

(a merge operation was necessary instead of simply omitting the error; see discussion at stan-dev#511)

fweber144 · 2025-05-20T18:32:30Z

With this PR branch, I get an error when using deltas="mixed" with more than stats
+ Error in plot.vsel(cvvs_fast, stats = c("elpd", "R2"), deltas = "mixed",  (from methods.R#771) : 
  Unexpected length of `stats_bs_init$value`. Please report this.
(e.g. using the example in the vignette)
Thanks; fixed in ee57c83.

That was a bit too hasty. I didn't realize that a merge operation was necessary (this was the reason why I added the error back then). Fixed now in b86caaa.

avehtari · 2025-05-21T07:25:01Z

deltas="mixed" does not work correctly for stats="gmpd" (EDIT: this used to work in my PR)

avehtari · 2025-05-21T07:45:03Z

With student case study https://users.aalto.fi/~ave/casestudies/VariableSelection/student.html, this worked in my PR

plot(vselm, stats=c("R2"), deltas=FALSE)

but with this PR gives an error

Error in sqrt_cut0((value_se^2 - 2 * mse_e/mse_y * cov_mse_e_y + (mse_e/mse_y)^2 *  (from misc.R#767) : 
  Negative (and numerically non-zero) value used as input to sqrt_cut0().

fweber144 · 2025-05-21T09:41:56Z

deltas="mixed" does not work correctly for stats="gmpd" (EDIT: this used to work in my PR)

Thanks; fixed in 69b349b.

fweber144 · 2025-05-21T14:15:56Z

With student case study https://users.aalto.fi/~ave/casestudies/VariableSelection/student.html, this worked in my PR
plot(vselm, stats=c("R2"), deltas=FALSE)
but with this PR gives an error
Error in sqrt_cut0((value_se^2 - 2 * mse_e/mse_y * cov_mse_e_y + (mse_e/mse_y)^2 *  (from misc.R#767) : 
  Negative (and numerically non-zero) value used as input to sqrt_cut0().

I get the same error with your PR. I guess you ran the code from your PR when commit f132e70 was not yet included in upstream/master (and in your PR branch neither, because I tried to keep your PR branch up-to-date with upstream/master). Note that the corresponding lines

projpred/R/summary_funs.R

Lines 441 to 443 in 0362770

    
           value_se <- sqrt_cut0((value_se^2 - 
        
                                    2 * mse_e / mse_y * cov_mse_e_y + 
        
                                    (mse_e / mse_y)^2 * var_mse_y) / mse_y^2)

have also been modified by #514, but that didn't change the behavior of those lines (sqrt_cut0() was added so that cutting off negative variances can be done at other places in the same manner).

Now the question is how we should fix that (on upstream/master). We could simply increase the tolerance used inside of sqrt_cut0(), which is currently at sqrt(.Machine$double.eps) (which is 1.5e-08 on my machine). In your case, we would have to increase it at least to 8.6e-06 (the value of abs(x) in your case), so 1e3 * sqrt(.Machine$double.eps) might make sense. Is that ok for you?

avehtari · 2025-05-22T06:28:11Z

1e3 * sqrt(.Machine$double.eps) might make sense. Is that ok for you?

ok

<#511 (comment)>

…_rewritten

fweber144 · 2025-05-23T11:23:22Z

1e3 * sqrt(.Machine$double.eps) might make sense. Is that ok for you?

ok

Fixed in 4daed93 (and merged upstream/master into this PR branch), so the issue should not occur anymore (I checked this on my machine).

…_rewritten

avehtari · 2025-06-03T12:50:39Z

I just tested plotting with all different stats using Portuguese student data (for continuos data) and Diabetes for binary data, and all did work well

…_rewritten

in the context of argument `deltas`, for consistency with the docs (and to differentiate this from the original response scale)

fweber144 · 2025-06-06T13:45:43Z

I just tested plotting with all different stats using Portuguese student data (for continuos data) and Diabetes for binary data, and all did work well

Thanks. I finished the open to-do items from above and will merge now.

of `summary.vsel()` from #511

implement deltas = "mixed" in plot.vsel(); originally,

e4381d3

this was proposed by @avehtari in stan-dev#496, but has been rewritten by @fweber144

fweber144 requested a review from avehtari April 23, 2025 10:51

fix parentheses in plot subtitle

fb424df

This was referenced May 3, 2025

Single-draw bug fix for augmented-data and latent projection #512

Merged

Dispersion in latent_ll_oscale() and latent_ppd_oscale() #513

Merged

fweber144 added 5 commits May 3, 2025 20:45

Merge remote-tracking branch 'upstream/master' into mixed_deltas_plot…

831e637

…_rewritten

Merge remote-tracking branch 'upstream/master' into mixed_deltas_plot…

8c4be15

…_rewritten

Merge remote-tracking branch 'upstream/master' into mixed_deltas_plot…

b81d3b0

…_rewritten

Merge remote-tracking branch 'upstream/master' into mixed_deltas_plot…

6b9e251

…_rewritten

Merge remote-tracking branch 'upstream/master' into mixed_deltas_plot…

b151013

…_rewritten

This was referenced May 19, 2025

Verbosity enhancements #518

Merged

Integer verbose (deprecate global options projpred.extra_verbose and projpred.verbose_project) #519

Merged

fweber144 added 3 commits May 19, 2025 22:37

Merge remote-tracking branch 'upstream/master' into mixed_deltas_plot…

d7ff874

…_rewritten

insert newline in plot subtitle; see

5638c09

<stan-dev#511 (comment)>

remove incorrect safety check; see

ee57c83

<stan-dev#511 (comment)> (the check is not necessary in the first place because `stats_bs_init` is a `data.frame`)

fixup! remove incorrect safety check; see

b86caaa

(a merge operation was necessary instead of simply omitting the error; see discussion at stan-dev#511)

fix deltas = "mixed" for the GMPD

69b349b

fweber144 added a commit that referenced this pull request May 23, 2025

increase the tolerance used inside of sqrt_cut0(), see

4daed93

<#511 (comment)>

Merge remote-tracking branch 'upstream/master' into mixed_deltas_plot…

b87c1ac

…_rewritten

Merge remote-tracking branch 'upstream/master' into mixed_deltas_plot…

d95c005

…_rewritten

fweber144 added 7 commits June 6, 2025 11:31

Merge remote-tracking branch 'upstream/master' into mixed_deltas_plot…

a3e4926

…_rewritten

update the main vignette to deltas = "mixed"

5fe3f7c

update the latent vignette to deltas = "mixed"

5a2551d

main vignette: use "actual scale" instead of "original scale"

de0d139

in the context of argument `deltas`, for consistency with the docs (and to differentiate this from the original response scale)

update remaining docs to deltas = "mixed"

d874d15

add NEWS.md entry

45cd517

add unit tests for deltas = "mixed"

5da3cab

fweber144 merged commit 8f63a6b into stan-dev:master Jun 6, 2025

fweber144 deleted the mixed_deltas_plot_rewritten branch June 6, 2025 13:46

fweber144 mentioned this pull request Jun 6, 2025

Add mixed deltas plot support #508

Closed

fweber144 added a commit that referenced this pull request Jun 7, 2025

add NEWS.md entry for the changes in argument type

f0e62cf

of `summary.vsel()` from #511

fweber144 added a commit that referenced this pull request Jun 15, 2025

minor cleaning in plot.vsel() (for code from #511)

b80ec0f

Uh oh!

Conversation

fweber144 commented Apr 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

avehtari commented May 18, 2025

Uh oh!

avehtari commented May 18, 2025

Uh oh!

avehtari commented May 18, 2025

Uh oh!

fweber144 commented May 20, 2025

Uh oh!

fweber144 commented May 20, 2025

Uh oh!

fweber144 commented May 20, 2025

Uh oh!

fweber144 commented May 20, 2025

Uh oh!

avehtari commented May 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

avehtari commented May 21, 2025

Uh oh!

fweber144 commented May 21, 2025

Uh oh!

fweber144 commented May 21, 2025

Uh oh!

avehtari commented May 22, 2025

Uh oh!

fweber144 commented May 23, 2025

Uh oh!

avehtari commented Jun 3, 2025

Uh oh!

fweber144 commented Jun 6, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

fweber144 commented Apr 23, 2025 •

edited

Loading

avehtari commented May 21, 2025 •

edited

Loading